Files that changed from the base of the PR and between a19e9ba and 4c47c15.

• packages/shader-lab/src/common/BaseScanner.ts (7 hunks)

Files that changed from the base of the PR and between d83cb29 and a180d29.

• packages/shader-lab/src/preprocessor/PpParser.ts (28 hunks)

packages/shader-lab/src/preprocessor/PpParser.ts (19)

Line range hint 25-40: Improved error handling and parsing context

The changes in this segment enhance the PpParser class by introducing new static properties for error tracking and parsing context. The use of conditional compilation for the _errors property is a good approach to provide detailed debugging information in verbose mode while keeping the release version optimized.

The switch from default export to named export is also a positive change, as it promotes better module management and explicit imports.

🧰 Tools

🪛 Biome

[error] 42-42: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

---

[error] 43-43: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

---

67-69: Improved parsing context and error handling

The changes to the parse method enhance error handling by allowing a null return to indicate parsing errors. Setting _scanningText and _scanningFile provides better context for error reporting throughout the parsing process.

Regarding the static analysis warning about using this in a static context: This is a valid use case in TypeScript for accessing static members within a class. If you prefer to make the code more explicit, you could replace this with PpParser, but it's not strictly necessary.

🧰 Tools

🪛 Biome

[error] 68-68: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

---

[error] 69-69: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

---

99-101: Enhanced error handling in verbose mode

This addition improves error handling in verbose mode by returning null when errors are encountered during parsing. This approach provides a clear signal to the caller that the parsing process failed, allowing for better error management and debugging in development environments.

---

110-114: Centralized error reporting

The new reportError method is a great addition that centralizes error reporting logic. This approach improves code maintainability and ensures consistent error object creation across the parser. It also makes it easier to modify error handling behavior in the future if needed.

---

Line range hint 140-148: Enhanced debugging information in verbose mode

The addition of BlockInfo creation and usage in verbose mode is a valuable improvement. This change provides more detailed context for debugging, allowing for better tracking of code blocks during the preprocessing stage. This extra information can be crucial for identifying and resolving complex issues in shader code.

---

Line range hint 168-178: Consistent enhancement of debugging information

This change follows the same pattern as previous additions, creating and using a BlockInfo object in verbose mode. This consistency in providing enhanced debugging information throughout the parser is commendable. It ensures that detailed context is available for all parsed blocks, which is crucial for effective debugging and error tracking in complex shader code.

---

Line range hint 203-213: Consistent approach to enhanced debugging

This change continues the pattern of adding detailed debugging information in verbose mode. The consistent use of BlockInfo objects across different parsing scenarios is praiseworthy. This approach ensures that comprehensive context is available throughout the entire parsing process, greatly aiding in debugging and error analysis for complex shader code.

---

221-232: Continued consistency in debugging enhancements

This change maintains the established pattern of adding detailed debugging information in verbose mode. The consistent approach to creating and using BlockInfo objects across various parsing scenarios is commendable. This uniformity in providing enhanced debugging context throughout the parser will significantly aid in troubleshooting and analyzing complex shader code issues.

---

Line range hint 235-246: Consistent implementation of debugging enhancements

This change follows the established pattern of adding detailed debugging information in verbose mode. The consistent creation and use of BlockInfo objects across different parsing scenarios demonstrate a thorough approach to enhancing the parser's debugging capabilities. This uniformity will greatly benefit developers when troubleshooting complex shader code issues.

---

309-310: Improved error reporting for relational expressions

This change enhances error reporting by using the centralized reportError method. The error message now provides more specific information about invalid operators in relational expressions. This improvement will make it easier for developers to identify and fix issues in shader code, particularly when dealing with complex expressions.

---

335-336: Consistent improvement in error reporting for shift expressions

This change continues the pattern of enhancing error reporting by using the centralized reportError method. The error message now provides specific information about invalid operators in shift expressions. This consistency in improving error reporting across different expression types will significantly aid developers in identifying and resolving issues in shader code.

---

359-360: Consistent enhancement of error reporting

This change maintains the pattern of improving error reporting by utilizing the centralized reportError method. The error message now provides specific information about invalid operators. This consistency in enhancing error reporting across various parts of the parser will greatly assist developers in quickly identifying and addressing issues in shader code.

---

381-382: Continued consistency in error reporting enhancements

This change follows the established pattern of improving error reporting by using the centralized reportError method. The error message provides specific information about invalid operators, maintaining consistency with previous improvements. This ongoing effort to enhance error reporting throughout the parser will significantly improve the debugging experience for developers working with shader code.

---

403-403: Consistent improvement in error reporting

This change maintains the pattern of enhancing error reporting by utilizing the centralized reportError method. The error message provides specific information about invalid operators, aligning with previous improvements. This consistency in improving error reporting across various parts of the parser will greatly aid developers in quickly identifying and resolving issues in shader code.

---

448-452: Enhanced error reporting for macro-related issues

These additions improve error reporting for macro-related issues by providing more specific error messages. The use of the reportError method for both invalid function macro usage and invalid const macro is consistent with the centralized error reporting approach. These enhancements will help developers quickly identify and resolve macro-related problems in shader code, improving the overall debugging experience.

---

461-466: Improved error reporting for invalid tokens

This addition enhances error reporting by providing a specific error message for invalid tokens encountered during parsing. The use of the reportError method is consistent with the centralized error reporting approach implemented throughout the parser. This improvement will help developers quickly identify and address syntax issues in shader code, further enhancing the debugging capabilities of the parser.

---

Line range hint 479-503: Enhanced debugging capabilities with source mapping

The addition of conditional compilation blocks for verbose mode in the method signatures and return types is a valuable improvement. Including the sourceMap property in verbose mode provides crucial debugging information, likely offering a mapping between the original and processed shader code. This enhancement will significantly aid developers in tracing issues back to their source in the original shader code, greatly improving the debugging experience for complex shader processing scenarios.

---

603-603: Consistent improvement in error reporting for macro redefinition

This change continues the pattern of enhancing error reporting by utilizing the centralized reportError method. The error message now provides specific information about redefined macros, particularly for those starting with "GL_". This consistency in improving error reporting across various parts of the parser, including macro handling, will greatly assist developers in identifying and resolving issues in shader code.

---

691-692: Comprehensive debugging enhancements for macro expansion

These changes consistently apply the pattern of adding detailed debugging information in verbose mode throughout the macro expansion process. The creation and use of BlockInfo objects, along with their inclusion in the expandSegments array, provide valuable context for debugging complex shader code.

This thorough approach to enhancing debugging capabilities across various stages of macro expansion will significantly aid developers in tracing and resolving issues related to macro processing in shader code. The consistency in implementing these improvements is commendable and will result in a more robust and developer-friendly preprocessing system.

Also applies to: 694-697, 699-701, 707-709, 712-714

Files that changed from the base of the PR and between a180d29 and 7c57e15.

• packages/shader-lab/src/index.ts (1 hunks)
• tests/src/shader-lab/Scanner.test.ts (1 hunks)

packages/shader-lab/src/index.ts (3)

3-5: LGTM: New internal export for BaseScanner added.

The addition of the BaseScanner export is correctly implemented. The @internal JSDoc tag appropriately indicates that this export is intended for internal use only.

---

16-17: LGTM: Mode initialization updated to "Verbose".

The change from "Editor" to "Verbose" mode under the _VERBOSE directive is consistent with the overall changes in this PR. This update aligns well with the improved error logging objectives.

---

7-9: Approve changes with a suggestion for directive consistency.

The changes align with the PR objectives, moving from _EDITOR to _VERBOSE directive and adding comprehensive error exports. However, ensure that all relevant parts of the codebase consistently use the _VERBOSE directive instead of _EDITOR.

To verify the consistent use of directives, run the following script:

✅ Verification successful

Adjust search commands to target TypeScript file extensions.

The previous rg commands failed because typescript is not a recognized file type. Please run the updated scripts below to verify the usage of _EDITOR and _VERBOSE directives within .ts and .tsx files.

---

Directive consistency verified.

All _EDITOR directives have been successfully replaced with _VERBOSE. No remaining _EDITOR directives found in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining _EDITOR directives that should be _VERBOSE

# Test: Search for _EDITOR directives. Expect: No results if all have been updated.
rg --type typescript '#if\s+_EDITOR'

# Test: Confirm _VERBOSE directives are used. Expect: Results showing _VERBOSE usage.
rg --type typescript '#if\s+_VERBOSE'

Length of output: 148

---

Script:

#!/bin/bash
# Description: Check for any remaining _EDITOR directives that should be _VERBOSE

# Test: Search for _EDITOR directives. Expect: No results if all have been updated.
rg '#if\s+_EDITOR' --glob '*.ts' --glob '*.tsx'

# Test: Confirm _VERBOSE directives are used. Expect: Results showing _VERBOSE usage.
rg '#if\s+_VERBOSE' --glob '*.ts' --glob '*.tsx'

Length of output: 10338

tests/src/shader-lab/Scanner.test.ts (1)

7-17: LGTM: Well-structured test case for skipping multi-line comments.

This test case effectively verifies that the skipCommentsAndSpace() method correctly handles multi-line comments and proceeds to the next token.

Files that changed from the base of the PR and between 7c57e15 and 4c63401.

• packages/shader-lab/src/codeGen/CodeGenVisitor.ts (3 hunks)
• packages/shader-lab/src/codeGen/GLESVisitor.ts (5 hunks)
• packages/shader-lab/src/contentParser/ShaderContentParser.ts (17 hunks)
• packages/shader-lab/src/parser/ShaderTargetParser.ts (3 hunks)
• packages/shader-lab/src/preprocessor/MacroDefine.ts (2 hunks)

packages/shader-lab/src/preprocessor/MacroDefine.ts (2)

3-4: LGTM: Updated imports align with new error handling approach.

The new imports for GSError, GSErrorName, and ShaderLabUtils are consistent with the improved error handling strategy mentioned in the PR objectives. The removal of ParserUtils import (as noted in the AI summary) also aligns with this change.

---

32-34: LGTM: Improved error handling for macro expansion.

The error handling in expandFunctionBody is well-implemented. It checks for argument count mismatch and throws a GSError with a clear message, error name, and location. This aligns with the PR objectives of improving error logging and provides valuable information for debugging.

packages/shader-lab/src/parser/ShaderTargetParser.ts (3)

11-17: Approve: Improved imports and conditional compilation.

The new imports and conditional compilation directives enhance the module's functionality and allow for efficient builds. The use of // #if _VERBOSE ensures that verbose error reporting is only included when necessary, potentially reducing the package size for non-verbose builds.

---

39-44: Approve: New errors getter with a reminder.

The addition of the errors getter is a good improvement for error reporting in verbose mode. The use of conditional compilation ensures it doesn't impact non-verbose builds.

However, there's still a potential typo in the property name sematicAnalyzer. Consider renaming it to semanticAnalyzer for clarity and consistency.

---

118-127: Approve: Removal of unreachable code.

The previously flagged unreachable code has been successfully removed, improving code clarity and addressing the issue raised in a past review comment. This change eliminates potential confusion and aligns with best practices for code maintenance.

packages/shader-lab/src/codeGen/GLESVisitor.ts (4)

9-9: LGTM: Import changes reflect updated dependencies

The addition of EKeyword import and removal of Logger import align with the changes in error handling approach. This modification suggests a shift towards using enum values and a different error reporting mechanism.

---

21-23: LGTM: Improved documentation

The addition of the @internal documentation comment enhances code clarity and follows best practices for documenting internal APIs.

---

32-34: LGTM: Enhanced error handling for verbose mode

The addition of the #if _VERBOSE block improves error handling capabilities in verbose mode, aligning with the PR objective of enhancing error logging for the Editor version.

---

Line range hint 1-160: Overall assessment: Changes align well with PR objectives

The modifications in this file successfully address the PR objectives by:

1. Improving error logging for ShaderLab compilation errors in the Editor version.
2. Implementing conditional compilation to support separate Editor and Release versions.
3. Enhancing type checking and error reporting in the vertexMain method.

These changes contribute to a more robust and maintainable codebase, balancing the trade-offs between performance optimization and code clarity as discussed in the PR comments.

packages/shader-lab/src/codeGen/CodeGenVisitor.ts (5)

3-3: LGTM: Appropriate imports added for error handling

The new imports for ShaderPosition, ShaderRange, GSErrorName, and GSError are correctly added and properly wrapped in preprocessor directives. This allows for conditional imports based on the build configuration, which aligns with the PR objectives of supporting different compilation modes.

Also applies to: 10-11

---

19-21: LGTM: Error collection mechanism added

The addition of the errors array as a readonly property is a good approach for collecting errors in verbose mode. This aligns well with the PR objectives of improving error logging for the Editor version.

---

45-50: LGTM: Improved error handling for attribute and varying structures

The changes to visitPostfixExpression method enhance error handling for attribute and varying structure references. The implementation is consistent and aligns with the PR objectives by collecting errors only in verbose mode.

Also applies to: 53-58

---

193-199: LGTM: Centralized error reporting mechanism

The new _reportError method effectively centralizes error reporting logic. It handles different compilation modes appropriately, creating a GSError in verbose mode and throwing a standard Error otherwise. This implementation aligns well with the PR objectives of supporting different compilation modes and improving error handling.

---

Line range hint 1-199: LGTM: Comprehensive improvements to error handling and reporting

The changes to CodeGenVisitor.ts successfully implement improved error handling and reporting mechanisms. The use of preprocessor directives to support different compilation modes is consistent and aligns well with the PR objectives. The new errors property and _reportError method provide a robust framework for collecting and reporting errors in the Editor version, while maintaining efficient compilation for the Release version.

These changes enhance the overall quality of the ShaderLab compilation process and should significantly improve the debugging experience for developers using the Editor version.

packages/shader-lab/src/contentParser/ShaderContentParser.ts (3)

26-29: LGTM: Enhanced error handling mechanism

The addition of GSError and GSErrorName imports, along with the new static _errors array, introduces a more structured approach to error handling. This improvement will facilitate better error reporting and debugging in the shader compilation process.

Also applies to: 56-57

---

69-69: LGTM: Proper error state reset

The addition of this._errors.length = 0; in the reset method ensures that the error collection is cleared before each parsing operation. This prevents the accumulation of stale errors across multiple parse calls, maintaining the integrity of error reporting.

🧰 Tools

🪛 Biome

[error] 69-69: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

---

117-117: LGTM: Improved position tracking

The replacement of scanner.curPosition with scanner.getCurPosition() across various methods is a positive change. Using a getter method ensures that the most up-to-date position is always retrieved, improving the accuracy of error reporting and code analysis.

Also applies to: 126-126, 133-133, 139-139, 157-157, 162-162, 354-354, 373-373, 382-382, 388-388, 396-396, 402-402, 420-420, 425-425, 464-464, 472-472, 478-478, 501-501, 519-519, 524-524

Files that changed from the base of the PR and between 4c63401 and 87c719d.

• packages/shader-lab/src/preprocessor/MacroDefine.ts (2 hunks)

packages/shader-lab/src/preprocessor/MacroDefine.ts (1)

3-6: Verify the conditional compilation approach for imports.

The GSErrorName import is wrapped in a conditional compilation block for verbose mode. This approach might lead to different behavior between verbose and non-verbose modes. Consider the following:

1. Ensure that GSErrorName is only used within conditionally compiled code blocks to prevent runtime errors in non-verbose mode.
2. Verify that the error handling logic remains consistent across both modes.

To check for potential issues, run the following script:

This will help identify any usage of GSErrorName that might cause issues in non-verbose mode.

✅ Verification successful

Conditional Compilation for GSErrorName Verified Successfully.

No usage of GSErrorName found outside the conditional compilation blocks, ensuring consistent behavior between verbose and non-verbose modes.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for GSErrorName usage outside of conditional compilation blocks

rg 'GSErrorName' --type typescript -g '!**/MacroDefine.ts' -C 5

Length of output: 208

Files that changed from the base of the PR and between 87c719d and eb515be.

• packages/shader-lab/src/preprocessor/MacroDefine.ts (1 hunks)

packages/shader-lab/src/preprocessor/MacroDefine.ts (2)

3-6: LGTM: New imports enhance error handling capabilities.

The addition of ShaderLabUtils and GSErrorName imports improves error handling. The conditional compilation for GSErrorName is a good practice for optimizing bundle size in non-verbose builds.

---

8-36: LGTM: Improved overall class structure.

The changes to the MacroDefine class have resulted in a more streamlined and efficient structure. The removal of the _expand method and the introduction of expandFunctionBody improve readability and maintainability.

The class now has a clearer separation of concerns, with the constructor handling initialization and expandFunctionBody focusing on the expansion logic.

Files that changed from the base of the PR and between eb515be and fb3974e.

• packages/shader-lab/src/GSError.ts (1 hunks)
• packages/shader-lab/src/ShaderLabUtils.ts (1 hunks)
• packages/shader-lab/src/codeGen/CodeGenVisitor.ts (3 hunks)
• packages/shader-lab/src/codeGen/VisitorContext.ts (3 hunks)
• packages/shader-lab/src/common/BaseScanner.ts (7 hunks)
• packages/shader-lab/src/contentParser/ShaderContentParser.ts (17 hunks)
• packages/shader-lab/src/parser/SemanticAnalyzer.ts (4 hunks)
• packages/shader-lab/src/parser/ShaderTargetParser.ts (3 hunks)
• packages/shader-lab/src/preprocessor/PpParser.ts (28 hunks)

• packages/shader-lab/src/parser/ShaderTargetParser.ts

packages/shader-lab/src/ShaderLabUtils.ts (4)

1-7: LGTM: Imports are well-organized and appropriate.

The imports are correctly structured, including both external and local modules. The conditional import for GSError aligns well with its usage in the code.

---

12-22: Approve createObjectPool and simplify clearAllShaderLabObjectPool.

The createObjectPool method is well-implemented and serves its purpose effectively.

For clearAllShaderLabObjectPool, consider simplifying the loop using a for...of statement as suggested in a previous review:

static clearAllShaderLabObjectPool() {
  for (const pool of ShaderLabUtils._shaderLabObjectPoolSet) {
    pool.clear();
  }
}

This change enhances readability and aligns with modern TypeScript practices.

---

24-36: LGTM: createGSError method is well-implemented.

The createGSError method effectively uses conditional compilation to provide different behavior based on the _VERBOSE flag. This approach allows for more detailed error information in verbose mode while keeping the release version lean.

Note: The static analysis tool flagged line 34 as unreachable code. This is a false positive due to the conditional compilation. In the final compiled code, only one of the conditional branches will exist, making all the code reachable.

🧰 Tools

🪛 Biome

[error] 34-34: This code is unreachable

... because this statement will return from the function beforehand

(lint/correctness/noUnreachable)

---

9-10: 🛠️ Refactor suggestion

Consider refactoring to a module with functions.

The ShaderLabUtils class currently only contains static members, which goes against best practices and triggers a static analysis warning. To improve code organization and maintainability, consider refactoring this into a module with exported functions instead of a class, as suggested in a previous review.

Here's a suggested refactoring:

import { ClearableObjectPool, IPoolElement } from "@galacean/engine";

const shaderLabObjectPoolSet: ClearableObjectPool<IPoolElement>[] = [];

export function createObjectPool<T extends IPoolElement>(type: new () => T) {
  // Implementation here
}

export function clearAllShaderLabObjectPool() {
  // Implementation here
}

export function createGSError(
  message: string,
  errorName: GSErrorName,
  source: string,
  location: ShaderRange | ShaderPosition,
  file?: string
) {
  // Implementation here
}

This approach maintains the same functionality while addressing the static analysis warning and potentially improving code readability and maintainability.

🧰 Tools

🪛 Biome

[error] 9-37: Avoid classes that contain only static members.

Prefer using simple functions instead of classes with only static members.

(lint/complexity/noStaticOnlyClass)

packages/shader-lab/src/GSError.ts (2)

1-4: LGTM: Preprocessor directive and imports look good.

The use of the _VERBOSE preprocessor directive is a good practice for controlling the inclusion of this file in different build configurations. The imports are relevant to the functionality of the GSError class.

---

60-65: LGTM: GSErrorName enum is well-defined.

The GSErrorName enum provides a clear categorization of error types. Its placement outside the _VERBOSE block is appropriate, making it available in all builds, which is likely necessary for use elsewhere in the codebase.

packages/shader-lab/src/parser/SemanticAnalyzer.ts (5)

3-3: LGTM: Import changes align with improved error handling.

The changes to the imports, including the addition of GSErrorName, ShaderLab, and the conditional import of GSError, are consistent with the PR objectives to enhance error handling and support different compilation modes.

Also applies to: 8-11

---

26-27: LGTM: Error array type updated for consistency.

The change from SemanticError[] to GSError[] for the errors array, along with the conditional compilation directive, is consistent with the overall changes in error handling and the introduction of verbose mode.

---

45-47: LGTM: Reset method updated for verbose mode.

The addition of the conditional compilation block to clear the errors array in the reset method is consistent with the changes in error handling for verbose mode.

---

68-72: LGTM: Improved error handling for verbose mode.

The changes to the error method for verbose mode align well with the PR objectives. The use of GSError with additional context from ShaderLab._processingPassText enhances error reporting. Returning the error instead of throwing it allows for more flexible error handling in the calling code.

---

73-74: ⚠️ Potential issue

Remove unreachable code in non-verbose mode.

The non-verbose error handling branch contains unreachable code due to the conditional compilation directive. This issue was also identified in previous reviews.

To resolve this, remove the entire #else branch as it will never be executed in the compiled code. Apply this diff:

 // #if _VERBOSE
    const err = new GSError(GSErrorName.CompilationError, param.join(""), loc, ShaderLab._processingPassText);
    this.errors.push(err);
    return err;
- // #else
-    throw new Error(param.join(""));
 // #endif

This change will eliminate the unreachable code warning while maintaining the intended functionality for both verbose and non-verbose modes.

🧰 Tools

🪛 Biome

[error] 73-73: This code is unreachable

... because this statement will return from the function beforehand

(lint/correctness/noUnreachable)

packages/shader-lab/src/codeGen/VisitorContext.ts (3)

5-11: LGTM: New imports enhance error handling and type safety

The new imports, including GSErrorName, BaseToken, ShaderLab, and ShaderLabUtils, are well-aligned with the changes in the referenceAttribute and referenceVarying methods. The conditional import of GSError for verbose mode is a good practice for managing different build configurations.

---

41-41: LGTM: Explicit private constructor

The addition of an explicit private constructor is a good practice. It reinforces the singleton pattern and makes the class's design intent clearer.

---

78-90: LGTM: Consistent improvements in error handling and type safety

The changes to referenceVarying method are consistent with those in referenceAttribute and offer similar benefits:

1. Using BaseToken instead of string provides more context for error reporting.
2. The new error handling approach using ShaderLabUtils.createGSError is more consistent and informative.
3. Early return for already referenced varyings is efficient.

Consider explicitly returning undefined when the varying is found for consistency:

-    if (this._referencedVaryingList[ident.lexeme]) return;
+    if (this._referencedVaryingList[ident.lexeme]) return undefined;

This change would make the method's behavior more explicit and consistent with its return type.

The static analysis tool indicates that line 83 is not covered by tests. Let's verify the test coverage for this method:

Please review the test coverage and consider adding tests to cover the error case in the referenceVarying method if they are missing.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 83-83: packages/shader-lab/src/codeGen/VisitorContext.ts#L83
Added line #L83 was not covered by tests

packages/shader-lab/src/codeGen/CodeGenVisitor.ts (5)

3-3: LGTM: Imports updated for new error handling functionality

The new imports for ShaderPosition, ShaderRange, GSErrorName, and GSError are correctly added to support the enhanced error handling capabilities. The conditional import of GSError based on the _VERBOSE flag aligns well with the PR objectives of creating distinct versions for Release and Editor.

Also applies to: 10-13

---

20-22: LGTM: New errors property for improved error logging

The addition of the readonly errors property of type GSError[] enhances the class's ability to collect and store errors during code generation in verbose mode. This change aligns perfectly with the PR objectives of improving error logging for the Editor version.

---

46-51: LGTM: Enhanced error handling in visitPostfixExpression

The additions to the visitPostfixExpression method significantly improve error handling for attribute and varying structure references. The conditional error collection based on the _VERBOSE flag aligns well with the PR objectives of implementing different behaviors for Release and Editor versions. These changes contribute to the overall improvement in error logging as mentioned in the PR summary.

Also applies to: 54-59

---

194-200: LGTM: Centralized error reporting with _reportError method

The new _reportError method effectively centralizes error reporting logic, enhancing code maintainability. Its conditional behavior based on the _VERBOSE flag aligns perfectly with the PR objectives of implementing different error handling for Release and Editor versions. This implementation is consistent with the PR summary's description of error handling improvements.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 196-196: packages/shader-lab/src/codeGen/CodeGenVisitor.ts#L196
Added line #L196 was not covered by tests

---

[warning] 198-198: packages/shader-lab/src/codeGen/CodeGenVisitor.ts#L198
Added line #L198 was not covered by tests

---

57-57: Improve test coverage for conditional compilation paths

The static analysis tool indicates that some lines in conditional blocks (57, 196, 198) are not covered by tests. These lines are crucial for the different behavior in Release and Editor versions. To ensure robustness and prevent potential regressions, consider adding tests that cover both compilation modes.

To verify the current test coverage and identify gaps, you can run the following command:

This script will help identify existing tests and potential gaps in coverage for the different compilation modes.

Also applies to: 196-196, 198-198

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 57-57: packages/shader-lab/src/codeGen/CodeGenVisitor.ts#L57
Added line #L57 was not covered by tests

packages/shader-lab/src/contentParser/ShaderContentParser.ts (1)

70-70: LGTM! Proper error state reset

Clearing the _errors array in the reset method ensures that errors from previous parsing operations don't persist. This is a good practice for maintaining the clean state of the parser between operations.

🧰 Tools

🪛 Biome

[error] 70-70: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

packages/shader-lab/src/preprocessor/PpParser.ts (6)

110-114: Approve centralized error reporting method

The addition of the reportError method is a positive change. It centralizes error reporting, improving consistency across the class and leveraging the new GSError type for more structured error handling.

This change enhances the overall error handling strategy of the class.

---

Line range hint 140-148: Approve enhanced debugging capabilities

The addition of the BlockInfo object and its inclusion in expandSegments for _VERBOSE mode enhances the debugging capabilities of the parser. This change provides more context for error reporting and debugging, which is valuable for development and troubleshooting.

These changes will significantly aid in diagnosing issues during development.

---

461-466: Approve improved error reporting for invalid tokens

The addition of error reporting for invalid tokens in constant parsing is a valuable improvement. Including the problematic character in the error message provides crucial information for debugging.

This change will make it easier to identify and fix issues related to unexpected tokens in constant expressions.

---

Line range hint 479-503: Approve enhanced debugging with source mapping

The addition of the sourceMap property to the return type in _VERBOSE mode is a valuable enhancement. This change will provide better context for mapping preprocessed code to the original source, significantly aiding in debugging and development processes.

These changes demonstrate a thoughtful approach to improving the development experience while maintaining a lean production build.

---

Line range hint 553-565: Approve consistent implementation of debugging enhancements

The addition of conditional compilation for _VERBOSE mode and the creation of BlockInfo objects across various methods demonstrate a consistent approach to enhancing debugging capabilities. These changes provide more detailed context for debugging and error tracking throughout the parsing process.

This consistent implementation will greatly improve the ability to diagnose and fix issues during development, while keeping the production build optimized.

Also applies to: 578-587, 616-625, 633-644

---

691-715: Approve improvements in range creation and debugging capabilities

The use of ShaderLab.createRange for creating range objects improves consistency throughout the parser. The addition of conditional compilation for _VERBOSE mode and the creation of BlockInfo objects further enhance the debugging capabilities of the parser.

These changes contribute to a more robust and developer-friendly parsing system, with improved consistency and better debugging tools.

packages/shader-lab/src/common/BaseScanner.ts (6)

28-31: Conditional compilation of _line and _column

Wrapping _line and _column under the _VERBOSE flag ensures that line and column tracking is only included in verbose builds. This optimizes performance by avoiding unnecessary property initialization in production environments.

---

41-46: Refactored getCurPosition() with conditional parameters

The modification of getCurPosition() to include _line and _column only when _VERBOSE is defined is appropriate. This ensures that position information is accurate in verbose mode while maintaining efficiency in production builds.

---

85-93: Optimized _advance() method with conditional line and column updates

Including the line and column updates within the _VERBOSE conditional block in the _advance() method is a good approach. It prevents unnecessary computations in non-verbose builds, enhancing performance.

---

136-136: Add unit tests for the new error handling logic

The invocation of throwError in scanText introduces a new error handling path that is not currently covered by tests, as indicated by the code coverage report. Adding unit tests for this code path will ensure that errors are handled correctly and improve overall test coverage.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 136-136: packages/shader-lab/src/common/BaseScanner.ts#L136
Added line #L136 was not covered by tests

---

141-144: Introduction of throwError method enhances error handling

The new throwError method centralizes error creation and throwing logic, improving maintainability and consistency in error handling throughout the BaseScanner class.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 142-143: packages/shader-lab/src/common/BaseScanner.ts#L142-L143
Added lines #L142 - L143 were not covered by tests

---

51-57: ⚠️ Potential issue

Potential access of undefined properties _line and _column

The getters line and column access _line and _column, which are only defined under the _VERBOSE flag. When _VERBOSE is not defined, accessing these properties will result in undefined or cause errors.

Consider wrapping the getters with the _VERBOSE conditional compilation to prevent access when the properties are not defined:

+  // #if _VERBOSE
  get line() {
    return this._line;
  }

  get column() {
    return this._column;
  }
+  // #endif

Likely invalid or redundant comment.

Files that changed from the base of the PR and between fb3974e and f0742cf.

• packages/shader-lab/src/common/BaseScanner.ts (7 hunks)

packages/shader-lab/src/common/BaseScanner.ts (6)

2-2: LGTM: New import for error handling.

The addition of GSErrorName import aligns with the PR objective of improving error logging for ShaderLab compilation errors.

---

51-59: LGTM: Added getters for line and column.

The new getters for line and column improve encapsulation and provide a clean interface for accessing these properties in verbose mode. The conditional compilation ensures these are only available when needed.

---

112-112: LGTM: Updated method call for consistency.

The change from curPosition to getCurPosition maintains consistency with the earlier method renaming.

Also applies to: 117-117, 119-119, 125-125

---

138-138: LGTM: Improved error handling in scanText.

The use of the new throwError method aligns with the PR objective of improving error handling. This change likely provides more detailed and consistent error reporting across the codebase.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 138-138: packages/shader-lab/src/common/BaseScanner.ts#L138
Added line #L138 was not covered by tests

---

176-176: LGTM: Updated method call for consistency.

The change from curPosition to getCurPosition maintains consistency with the earlier method renaming.

Also applies to: 179-179

---

87-95: LGTM: Simplified line and column tracking.

Removing the conditional compilation around line and column updates simplifies the code and ensures consistent tracking. However, consider the performance impact of always updating these values, especially for large files.

You might want to benchmark this change to ensure it doesn't significantly affect performance in non-verbose mode.

✅ Verification successful

Verified: Line and column tracking consistent.

The removal of conditional compilation around _line and _column in BaseScanner.ts ensures consistent tracking, aligning with their usage in Lexer.ts. No additional impacts detected based on the current codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the usage of _line and _column outside of _VERBOSE mode
rg --type typescript -g '!*.test.ts' '\b_line\b|\b_column\b' packages/shader-lab/src

Length of output: 121

---

Script:

#!/bin/bash
# Verify the usage of _line and _column outside of _VERBOSE mode
rg --type ts -g '!*.test.ts' '\b_line\b|\b_column\b' packages/shader-lab/src

Length of output: 895

---

Script:

#!/bin/bash
# Examine the context of _line and _column usage in Lexer.ts and other relevant files
ast-grep --lang typescript --pattern $'_line = $_' packages/shader-lab/src/lexer/Lexer.ts
ast-grep --lang typescript --pattern $'_column = $_' packages/shader-lab/src/lexer/Lexer.ts

Length of output: 183

---

Script:

#!/bin/bash
# Search for assignments to _line and _column in Lexer.ts and BaseScanner.ts
rg --type ts -g '!*.test.ts' 'this\._line\s*=' packages/shader-lab/src/lexer/Lexer.ts
rg --type ts -g '!*.test.ts' 'this\._column\s*=' packages/shader-lab/src/lexer/Lexer.ts
rg --type ts -g '!*.test.ts' 'this\._line\s*=' packages/shader-lab/src/common/BaseScanner.ts
rg --type ts -g '!*.test.ts' 'this\._column\s*=' packages/shader-lab/src/common/BaseScanner.ts

Length of output: 463